test: codrabbit prompt#211
Conversation
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
feat: proto prompt
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
test: publish proto2
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
WalkthroughRefactors AccountInfo by replacing legacy staking fields with a composite Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (1 warning, 3 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| f"staked_info={self.staking_info!r}, " | ||
| f")" |
There was a problem hiding this comment.
Use staking_info label in repr for consistency.
The debug string currently says staked_info, which doesn’t match the field name.
✅ Proposed fix
- f"staked_info={self.staking_info!r}, "
+ f"staking_info={self.staking_info!r}, "📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| f"staked_info={self.staking_info!r}, " | |
| f")" | |
| f"staking_info={self.staking_info!r}, " | |
| f")" |
| def test_from_proto_with_staking_info(): | ||
| """Test the from_proto method of the AccountInfo class with staking info""" | ||
| public_key = PrivateKey.generate_ed25519().public_key() | ||
|
|
||
| staking_info={ | ||
| "decline_reward": True, | ||
| "staked_node_id": 3, | ||
| "staked_account_id": None | ||
| } | ||
|
|
||
| proto = CryptoGetInfoResponse.AccountInfo( | ||
| accountID=AccountId(0, 0, 100)._to_proto(), | ||
| key=public_key._to_proto(), | ||
| balance=5000000, | ||
|
|
||
|
|
||
| ) | ||
|
|
||
| account_info = AccountInfo._from_proto(proto) | ||
|
|
||
| assert account_info.staking_info is not None | ||
| assert account_info.staking_info.decline_reward is True | ||
| assert account_info.staking_info.staked_node_id == 3 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the test file
find . -name "account_info_test.py" -type fRepository: exploreriii/hiero_sdk_python
Length of output: 105
🏁 Script executed:
# List the test/unit directory structure
ls -la tests/unit/ | head -20Repository: exploreriii/hiero_sdk_python
Length of output: 1513
🏁 Script executed:
# Search for StakingInfo class definition
rg "class StakingInfo" --type pyRepository: exploreriii/hiero_sdk_python
Length of output: 127
🏁 Script executed:
# Search for the AccountInfo class and its _from_proto method
rg "def _from_proto" -A 15 --type py | grep -A 15 "AccountInfo"Repository: exploreriii/hiero_sdk_python
Length of output: 2336
🏁 Script executed:
# Read the test file to verify the test at lines 76-99
cat -n tests/unit/account_info_test.py | sed -n '70,105p'Repository: exploreriii/hiero_sdk_python
Length of output: 1561
🏁 Script executed:
# Read the full AccountInfo._from_proto implementation
cat -n src/hiero_sdk_python/account/account_info.py | grep -A 50 "def _from_proto"Repository: exploreriii/hiero_sdk_python
Length of output: 2713
🏁 Script executed:
# Check the StakingInfo class definition and _to_proto method
cat -n src/hiero_sdk_python/staking_info.pyRepository: exploreriii/hiero_sdk_python
Length of output: 6189
Populate proto.staking_info so the test actually exercises _from_proto's staking_info deserialization.
The staking_info dict is unused; _from_proto checks proto.HasField("staking_info") which will be False, causing account_info.staking_info to be None and failing the assertions.
Fix
- staking_info={
- "decline_reward": True,
- "staked_node_id": 3,
- "staked_account_id": None
- }
+ staking_info = StakingInfo(
+ decline_reward=True,
+ staked_node_id=3,
+ staked_account_id=None,
+ )
proto = CryptoGetInfoResponse.AccountInfo(
accountID=AccountId(0, 0, 100)._to_proto(),
key=public_key._to_proto(),
balance=5000000,
-
-
)
+ proto.staking_info.CopyFrom(staking_info._to_proto())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_from_proto_with_staking_info(): | |
| """Test the from_proto method of the AccountInfo class with staking info""" | |
| public_key = PrivateKey.generate_ed25519().public_key() | |
| staking_info={ | |
| "decline_reward": True, | |
| "staked_node_id": 3, | |
| "staked_account_id": None | |
| } | |
| proto = CryptoGetInfoResponse.AccountInfo( | |
| accountID=AccountId(0, 0, 100)._to_proto(), | |
| key=public_key._to_proto(), | |
| balance=5000000, | |
| ) | |
| account_info = AccountInfo._from_proto(proto) | |
| assert account_info.staking_info is not None | |
| assert account_info.staking_info.decline_reward is True | |
| assert account_info.staking_info.staked_node_id == 3 | |
| def test_from_proto_with_staking_info(): | |
| """Test the from_proto method of the AccountInfo class with staking info""" | |
| public_key = PrivateKey.generate_ed25519().public_key() | |
| staking_info = StakingInfo( | |
| decline_reward=True, | |
| staked_node_id=3, | |
| staked_account_id=None, | |
| ) | |
| proto = CryptoGetInfoResponse.AccountInfo( | |
| accountID=AccountId(0, 0, 100)._to_proto(), | |
| key=public_key._to_proto(), | |
| balance=5000000, | |
| ) | |
| proto.staking_info.CopyFrom(staking_info._to_proto()) | |
| account_info = AccountInfo._from_proto(proto) | |
| assert account_info.staking_info is not None | |
| assert account_info.staking_info.decline_reward is True | |
| assert account_info.staking_info.staked_node_id == 3 |
🧰 Tools
🪛 GitHub Actions: Code Coverage
[error] 96-96: pytest failure in test_from_proto_with_staking_info: staking_info is None but expected not None. This indicates AccountInfo._from_proto is not populating staking_info when staking info is provided.
🪛 Ruff (0.14.14)
[error] 80-80: Local variable staking_info is assigned to but never used
Remove assignment to unused variable staking_info
(F841)
1 participant
Description:
Related issue(s):
Fixes #1
Notes for reviewer:
Checklist